Conversation
h3adex
force-pushed
the
feat/add-rt-support
branch
from
d8f4c9f to
60fae5d
Compare
h3adex
force-pushed
the
feat/add-rt-support
branch
from
7a76aaa to
565f907
Compare
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
h3adex
force-pushed
the
feat/add-rt-support
branch
from
2d82ab3 to
d4e9329
Compare
h3adex
force-pushed
the
feat/add-rt-support
branch
from
d4e9329 to
4d50a2b
Compare
| Description: utils.Ptr(testRoutingTableDescription), | ||
| SystemRoutes: testSystemRoutesFlag, | ||
| DynamicRoutes: testDynamicRoutesFlag, | ||
| Labels: utils.Ptr(*testLabels), |
There was a problem hiding this comment.
can be simplified
| Labels: utils.Ptr(*testLabels), | |
| Labels: testLabels, |
| Args: args.SingleArg(routingTableIdArg, utils.ValidateUUID), | ||
| Example: examples.Build( | ||
| examples.NewExample( | ||
| `Deletes a a routing-table`, |
There was a problem hiding this comment.
| `Deletes a a routing-table`, | |
| `Delete a routing-table with ID "xxx"`, |
| description: "invalid routing-table ID - empty", | ||
| argValues: []string{testRoutingTableId}, | ||
| flagValues: fixtureFlagValues(func(flagValues map[string]string) { | ||
| flagValues[routingTableIdArg] = "" |
There was a problem hiding this comment.
this test is failing because the flag routingTableIdArg doesn't exist. the routing-table id is actually set here in the argValues.
| } | ||
|
|
||
| // Next Hop validation logic | ||
| switch strings.ToLower(*model.NextHopType) { |
There was a problem hiding this comment.
I would set the type of NextHopType to string instead *string. The flag is marked as required so it should be always set. When it's set to a pointer string, it could more likely result somewhere in a nil pointer. With the string type we can prevent this.
|
|
||
| // Next Hop validation logic | ||
| switch strings.ToLower(*model.NextHopType) { | ||
| case "internet", "blackhole": |
There was a problem hiding this comment.
please add some consts for the different nexthopeTypes instead of writing them all as plain strings. this can prevent future issues if somewhere is a typo
| if len(routes) == 0 { | ||
| return fmt.Errorf("create routes response is empty") | ||
| } | ||
|
|
||
| return p.OutputResult(outputFormat, routes, func() error { | ||
| table := tables.NewTable() | ||
| table.SetHeader("ID", "DESTINATION TYPE", "DESTINATION VALUE", "NEXTHOP TYPE", "NEXTHOP VALUE", "LABELS", "CREATED AT", "UPDATED AT") | ||
| for _, route := range routes { | ||
| routeDetails := routeUtils.ExtractRouteDetails(route) | ||
|
|
||
| table.AddRow( | ||
| utils.PtrString(route.Id), | ||
| routeDetails.DestType, | ||
| routeDetails.DestValue, | ||
| routeDetails.HopType, | ||
| routeDetails.HopValue, | ||
| routeDetails.Labels, | ||
| routeDetails.CreatedAt, | ||
| routeDetails.UpdatedAt, | ||
| ) | ||
| } | ||
| err := table.Display(p) | ||
| if err != nil { | ||
| return fmt.Errorf("render table: %w", err) | ||
| } | ||
| return nil | ||
| }) |
There was a problem hiding this comment.
Same as mentioned by Ruben in the routingtable create command, we usually don't print a table in the create command. Instead a small info message is enough, e.g. "Created route with ID xxx in routing table yyy"
| isValid: true, | ||
| }, | ||
| { | ||
| description: "argument value missing", |
There was a problem hiding this comment.
This command actually doesn't support arguments. So if the argument would be really missing, it should pass
| description: "argument value missing", | |
| description: "argument value is empty string", |
| return err | ||
| } | ||
|
|
||
| prompt := fmt.Sprintf("Are you sure you want to delete the route %q in routing-table %q for network-area-id %q?", model.RouteID, model.RoutingTableId, model.OrganizationId) |
There was a problem hiding this comment.
| prompt := fmt.Sprintf("Are you sure you want to delete the route %q in routing-table %q for network-area-id %q?", model.RouteID, model.RoutingTableId, model.OrganizationId) | |
| prompt := fmt.Sprintf("Are you sure you want to delete the route %q in routing-table %q for network area id %q?", model.RouteID, model.RoutingTableId, model. NetworkAreaId) |
3 participants
Description
reopened #1023
Checklist
make fmtmake generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)